Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[unitaryHACK] Add reset error to noise channels #1321

Merged
merged 25 commits into from
May 20, 2021

Conversation

nahumsa
Copy link
Contributor

@nahumsa nahumsa commented May 15, 2021

Context: Add Reset Error to noise channels

Description of the Change: Add Single-qubit Reset error channel to pennylane.

Benefits: Add a new noise model that simulates the reset of qubits to 0 or 1 given a probability.

Possible Drawbacks:

Related GitHub Issues: #971

@nahumsa nahumsa changed the title [WIP] [unitaryhack] Add reset error to noise channels [unitaryhack] Add reset error to noise channels May 15, 2021
@josh146 josh146 added the unitaryhack Dedicated issue for Unitary Fund open-source hackathon label May 17, 2021
@josh146 josh146 requested a review from ixfoduap May 17, 2021 06:43
@josh146
Copy link
Member

josh146 commented May 17, 2021

Hi @nahumsa! Thanks for this PR -- this is a great issue to tackle. Just a bit of housekeeping before we can provide a review:

  • The formatting check is failing. All that is required is that the PennyLane source code and tests folder is 'blacked' using the Black formatting tool:

    pip install black
    black -l 100 pennylane
    black -l 100 tests
  • The documentation check is also failing, due to a missing space after the backtick in the docstring:

    :math:`p_1 \in [0, 1]`is
    

    You can click on the 'details' link beside the docs/readthedocs.org:pennylane CI check to view what the rendered documentation for your PR looks like, to make sure it looks okay when built :)

    image

  • Finally, it looks like there is an issue with the new tests, causing the test CI to fail.

Let us know when these have been addressed, and the PR is ready for review! Alternatively, feel free to ask any questions.

@nahumsa
Copy link
Contributor Author

nahumsa commented May 17, 2021

Thanks for the response, @josh146 ! I will fix those issues and let you know when it's ready for a review.

Changed the probabilities of tests, because the sum of probs was greater than 1, and fix the gradient circuit parameters
@nahumsa
Copy link
Contributor Author

nahumsa commented May 17, 2021

I think that this is ready for review.

Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this great PR @nahumsa! In particular, fantastic tests.

However, it doesn't appear that the tests are passing; they are giving the error

AssertionError: Gradient recipe must have one entry for each parameter!

The grad_recipe you have defined provides a gradient recipe only for the first parameter:

grad_recipe = ([[1, 0, 1], [-1, 0, 0]],)

You will need to also provide the gradient recipe for the second parameter, p_1. Let me know if you have any questions!

.github/CHANGELOG.md Outdated Show resolved Hide resolved
Comment on lines 313 to 319
@classmethod
def _kraus_matrices(cls, *params):
p_0, p_1 = params[0], params[1]
K0 = np.sqrt(1 - p_0 - p_1) * np.eye(2)
K1 = np.sqrt(p_0) * np.array([[1, 0], [0, 0]])
K2 = np.sqrt(p_1) * np.array([[0, 0], [0, 1]])
return [K0, K1, K2]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Comment on lines 236 to 252
@pytest.mark.parametrize("angle", np.linspace(0, 2 * np.pi, 7))
def test_grad_reset(self, angle, tol):
"""Test that analytical gradient is computed correctly for different states. Channel
grad recipes are independent of channel parameter"""

dev = qml.device("default.mixed", wires=1)
p_0, p_1 = 0.5, 0.5

@qml.qnode(dev)
def circuit(p_0, p_1):
qml.RX(angle, wires=0)
qml.ResetError(p_0, p_1, wires=0)
return qml.expval(qml.PauliZ(0))

gradient = np.squeeze(qml.grad(circuit)(p_0, p_1))
assert gradient == circuit(1) - circuit(0)
assert np.allclose(gradient, (-2 * np.cos(angle)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great test @nahumsa! Did you calculate the expected gradient value, -2 cos(angle), by hand?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not sure about the analytical gradients for this one, I was using TestBitFlip as a reference.
Maybe could you clarify a bit more about the gradient of this channel?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the best sources for learning more about the analytic gradients of noisy channels is https://johannesjakobmeyer.com/blog/004-noisy-parameter-shift/, or alternatively, the PennyLane demonstration: https://pennylane.ai/qml/demos/tutorial_noisy_circuit_optimization.html

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this particular case, I think @ixfoduap calculated the analytic value by hand, so hopefully he will be able to shed some light!

Copy link
Contributor Author

@nahumsa nahumsa May 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the followup. I'll read those references.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nahumsa, this is a great PR!

I'm not sure about the analytical gradients. The way we calculate analytical gradients is using this formula, which also appears in the blog post that Josh linked to:
Screenshot from 2021-05-18 14-45-22

It is valid when the channel can be expressed as a linear combination of channels. This typically means that the Kraus matrices themselves should be a valid channel, which in practice often boils down to the Kraus matrices being unitaries.

In your case, the projectors |0><0| and |1><1| are not by themselves valid channels, so it's not accurate that the reset channel can be expressed as a linear combination of channels. This means that the analytical formula should not apply here.

I believe that the reason the tests are passing is because you are setting both p_0 and p_1 equal to each other, which effectively makes this the identity channel --> sqrt(p) |0><0| + sqrt(p) |1><1| = sqrt(p) (|0><0|+|1><1|) = sqrt(p) \identity and we end up with the gradient of the gate in the circuit.

I would expect the test to fail if you make p_0 != p_1 and compute the gradient by hand there. It's worth checking that this is the case. In any case, the gradient test should be checking gradients for many values of p_0 and p_1.

If the analytical gradients do not apply here, it is not a big deal; it just means you should change the gradient method to "F" (finite difference) like it is done for other channels.

Copy link
Contributor Author

@nahumsa nahumsa May 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ixfoduap! Thanks for the clarification. After reading the reference, I think that it is not possible to get analytic gradient by the formulas used on other channels.
Thanks for the catch about the tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ixfoduap!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nahumsa let us know when you have the tests passing, or if you need a hand anywhere :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josh146 and @ixfoduap, after revising some math, I found that I missed two Kraus operators that constitutes the ResetError channel! Sorry about that, I always find quantum channels quite tricky 😅

@josh146
Copy link
Member

josh146 commented May 17, 2021

Oh, and don't forget to add your name to the Contributors section of the changelog!

Co-authored-by: Josh Izaac <josh146@gmail.com>
@nahumsa
Copy link
Contributor Author

nahumsa commented May 17, 2021

Oh, and don't forget to add your name to the Contributors section of the changelog!

Sure! :)

Comment on lines 236 to 252
@pytest.mark.parametrize("angle", np.linspace(0, 2 * np.pi, 7))
def test_grad_reset(self, angle, tol):
"""Test that analytical gradient is computed correctly for different states. Channel
grad recipes are independent of channel parameter"""

dev = qml.device("default.mixed", wires=1)
p_0, p_1 = 0.5, 0.5

@qml.qnode(dev)
def circuit(p_0, p_1):
qml.RX(angle, wires=0)
qml.ResetError(p_0, p_1, wires=0)
return qml.expval(qml.PauliZ(0))

gradient = np.squeeze(qml.grad(circuit)(p_0, p_1))
assert gradient == circuit(1) - circuit(0)
assert np.allclose(gradient, (-2 * np.cos(angle)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nahumsa, this is a great PR!

I'm not sure about the analytical gradients. The way we calculate analytical gradients is using this formula, which also appears in the blog post that Josh linked to:
Screenshot from 2021-05-18 14-45-22

It is valid when the channel can be expressed as a linear combination of channels. This typically means that the Kraus matrices themselves should be a valid channel, which in practice often boils down to the Kraus matrices being unitaries.

In your case, the projectors |0><0| and |1><1| are not by themselves valid channels, so it's not accurate that the reset channel can be expressed as a linear combination of channels. This means that the analytical formula should not apply here.

I believe that the reason the tests are passing is because you are setting both p_0 and p_1 equal to each other, which effectively makes this the identity channel --> sqrt(p) |0><0| + sqrt(p) |1><1| = sqrt(p) (|0><0|+|1><1|) = sqrt(p) \identity and we end up with the gradient of the gate in the circuit.

I would expect the test to fail if you make p_0 != p_1 and compute the gradient by hand there. It's worth checking that this is the case. In any case, the gradient test should be checking gradients for many values of p_0 and p_1.

If the analytical gradients do not apply here, it is not a big deal; it just means you should change the gradient method to "F" (finite difference) like it is done for other channels.

@nahumsa nahumsa changed the title [unitaryhack] Add reset error to noise channels [unitaryHACK] Add reset error to noise channels May 19, 2021
@codecov
Copy link

codecov bot commented May 19, 2021

Codecov Report

Merging #1321 (b35a7be) into master (98a7876) will decrease coverage by 0.94%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1321      +/-   ##
==========================================
- Coverage   99.10%   98.16%   -0.95%     
==========================================
  Files         141      145       +4     
  Lines       10575    11093     +518     
==========================================
+ Hits        10480    10889     +409     
- Misses         95      204     +109     
Impacted Files Coverage Δ
pennylane/ops/channel.py 100.00% <100.00%> (ø)
pennylane/tape/operation_recorder.py 64.51% <0.00%> (-35.49%) ⬇️
pennylane/measure.py 91.76% <0.00%> (-8.24%) ⬇️
pennylane/circuit_drawer/grid.py 88.70% <0.00%> (-7.72%) ⬇️
pennylane/grouping/optimize_measurements.py 92.30% <0.00%> (-7.70%) ⬇️
pennylane/vqe/vqe.py 92.97% <0.00%> (-7.03%) ⬇️
pennylane/configuration.py 93.10% <0.00%> (-6.90%) ⬇️
pennylane/qnn/torch.py 95.08% <0.00%> (-4.92%) ⬇️
pennylane/qnn/keras.py 95.38% <0.00%> (-4.62%) ⬇️
pennylane/about.py 95.45% <0.00%> (-4.55%) ⬇️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98a7876...b35a7be. Read the comment docs.

Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great contribution @nahumsa, looks ready to merge on my end! I might wait to see if @ixfoduap has any requests.

@josh146 josh146 requested a review from ixfoduap May 19, 2021 14:57
\end{bmatrix}

.. math::
K_2 = \sqrt{p_0}\begin{bmatrix}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nahumsa Why did you change the definition of the channel?
Before approving, could you please point me to a reference where this channel is defined? I would like to make sure that the name "reset error" coincides with the standard usage. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ixfoduap ! I was reading through the qiskit's docs and found a direct reference about the ResetError Kraus operators:
image

This was the main reason that I changed the definition of the channel, do you think that this is reasonable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! Thanks for the clarification!

Copy link
Contributor

@ixfoduap ixfoduap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@josh146 josh146 merged commit 24e9360 into PennyLaneAI:master May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unitaryhack Dedicated issue for Unitary Fund open-source hackathon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants